Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a note about the known limitations of the credential claim metadata #280

Closed
wants to merge 1 commit into from

Conversation

jogu
Copy link
Contributor

@jogu jogu commented Feb 28, 2024

As discussed on yesterday's working group call:

#266 (comment)

As it seems likely that #276 will not make it into implementer's draft 1, we instead add warnings about the known limitations of the current data structure so that implementer's are aware.

Note that I have not applied this to the mdoc profile section, as that sections seems not to allow nested objects in the metadata.

As discussed on yesterday's working group call:

#266 (comment)

As it seems likely that #276
will not make it into implementer's draft 1, we instead add warnings
about the known limitations of the current data structure so that
implementer's are aware.

Note that I have not applied this to the mdoc profile section, as that
sections seems not to allow nested objects in the metadata.
@jogu
Copy link
Contributor Author

jogu commented Feb 28, 2024

It case it's helpful here's an example of how this renders:

Screenshot 2024-02-28 at 13 56 44

> The above metadata structure has some known limitations:
>
> * It cannot be used to describe claims in credentials that have the name `mandatory`, `value_type` or `display`.
> * It is not possible to provide `mandatory`, `value_type` or `display` values for objects that contain claims
Copy link
Collaborator

@Sakurann Sakurann Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the most important and should be the first one. and it should also be much more clearer, something like:

Suggested change
> * It is not possible to provide `mandatory`, `value_type` or `display` values for objects that contain claims
> * It is not possible to provide `mandatory`, `value_type` or `display` values for the intermediary claims in the nested objects

Comment on lines +1998 to +2006
> The above metadata structure has some known limitations:
>
> * It cannot be used to describe claims in credentials that have the name `mandatory`, `value_type` or `display`.
> * It is not possible to provide `mandatory`, `value_type` or `display` values for objects that contain claims
> * The `order` parameter cannot be used for claims within objects
> * Arrays of unknown size cannot be described
>
> These limitations are expected to be resolved in the second Implementer's Draft, a proposal can be viewed in [Issue 266](https://github.com/openid/OpenID4VCI/issues/266).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have not used this > format in other places in the spec. I would like to avoid introducing it and drawing unnecessary attention to this

Suggested change
> The above metadata structure has some known limitations:
>
> * It cannot be used to describe claims in credentials that have the name `mandatory`, `value_type` or `display`.
> * It is not possible to provide `mandatory`, `value_type` or `display` values for objects that contain claims
> * The `order` parameter cannot be used for claims within objects
> * Arrays of unknown size cannot be described
>
> These limitations are expected to be resolved in the second Implementer's Draft, a proposal can be viewed in [Issue 266](https://github.com/openid/OpenID4VCI/issues/266).
Note that the above metadata structure has some known limitations:
* It cannot be used to describe claims in credentials that have the name `mandatory`, `value_type` or `display`.
* It is not possible to provide `mandatory`, `value_type` or `display` values for objects that contain claims
* The `order` parameter cannot be used for claims within objects
* Arrays of unknown size cannot be described
These limitations are expected to be resolved in the second Implementer's Draft, a proposal can be viewed in [Issue 266](https://github.com/openid/OpenID4VCI/issues/266).

> * The `order` parameter cannot be used for claims within objects
> * Arrays of unknown size cannot be described
>
> These limitations are expected to be resolved in the second Implementer's Draft, a proposal can be viewed in [Issue 266](https://github.com/openid/OpenID4VCI/issues/266).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about the current framing... I think the following is sufficient.

Suggested change
> These limitations are expected to be resolved in the second Implementer's Draft, a proposal can be viewed in [Issue 266](https://github.com/openid/OpenID4VCI/issues/266).
> Mechanisms that could address these limitations are being discussed in [Issue 266](https://github.com/openid/OpenID4VCI/issues/266).

Copy link
Collaborator

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reading the PR, honestly, not sure this should be in the ID-1 text...

made some suggestions still..

@jogu
Copy link
Contributor Author

jogu commented Mar 4, 2024

This was discussed on last Thursday's working group call and there wasn't anyone that felt strongly about merging this for ID2. I suggest it can probably be closed.

@jogu
Copy link
Contributor Author

jogu commented Apr 2, 2024

closing as we decided not to include this before ID1.

@jogu jogu closed this Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants